Skip to content

Conversation

@arsenm
Copy link
Contributor

@arsenm arsenm commented Aug 18, 2025

Verify it's a multiple of the result vector element count
instead of asserting this in random combines.

The testcase in #153808 fails in the wrong point. Add an
assert to getNode so the invalid extract asserts at construction
instead of use.

Copy link
Contributor Author

arsenm commented Aug 18, 2025

@llvmbot
Copy link
Member

llvmbot commented Aug 18, 2025

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

Verify it's a multiple of the result vector element count
instead of asserting this in random combines.

The testcase in #153808 fails in the wrong point. Add an
assert to getNode so the invalid extract asserts at construction
instead of use.


Full diff: https://github.com/llvm/llvm-project/pull/154099.diff

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (-2)
  • (modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+2)
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 785245b2d9e74..a8985b0a0fd56 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -26094,8 +26094,6 @@ SDValue DAGCombiner::visitEXTRACT_SUBVECTOR(SDNode *N) {
     EVT ConcatSrcVT = V.getOperand(0).getValueType();
     assert(ConcatSrcVT.getVectorElementType() == NVT.getVectorElementType() &&
            "Concat and extract subvector do not change element type");
-    assert((ExtIdx % ExtNumElts) == 0 &&
-           "Extract index is not a multiple of the input vector length.");
 
     unsigned ConcatSrcNumElts = ConcatSrcVT.getVectorMinNumElements();
     unsigned ConcatOpIdx = ExtIdx / ConcatSrcNumElts;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 84282d8a1c37b..fadf2c7a4b9bc 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -7956,6 +7956,8 @@ SDValue SelectionDAG::getNode(unsigned Opcode, const SDLoc &DL, EVT VT,
     assert(N2C->getAPIntValue().getBitWidth() ==
                TLI->getVectorIdxWidth(getDataLayout()) &&
            "Constant index for EXTRACT_SUBVECTOR has an invalid size");
+    assert(N2C->getZExtValue() % VT.getVectorMinNumElements() == 0 &&
+           "Extract index is not a multiple of the output vector length");
 
     // Trivial extraction.
     if (VT == N1VT)

@arsenm arsenm marked this pull request as ready for review August 18, 2025 11:36
arsenm added 3 commits August 19, 2025 00:30
Verify it's a multiple of the result vector element count
instead of asserting this in random combines.

The testcase in #153808 fails in the wrong point. Add an
assert to getNode so the invalid extract asserts at construction
instead of use.
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arsenm arsenm force-pushed the users/arsenm/amdgpu/fix-illegal-extract-subvector-split-vector branch from 7bdd4ac to ce7657b Compare August 19, 2025 23:52
@arsenm arsenm force-pushed the users/arsenm/dag/add-assert-getNode-EXTRACT_SUBVECTOR-index-multiple-vector-elts branch from e092268 to 3fd9c57 Compare August 19, 2025 23:52
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Base automatically changed from users/arsenm/amdgpu/fix-illegal-extract-subvector-split-vector to main August 20, 2025 00:35
@arsenm arsenm merged commit 276c1d8 into main Aug 20, 2025
12 of 15 checks passed
@arsenm arsenm deleted the users/arsenm/dag/add-assert-getNode-EXTRACT_SUBVECTOR-index-multiple-vector-elts branch August 20, 2025 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants